backends/advanced/src/advanced_omi_backend/controllers/websocket_controller.py-753-760 (1)

753-760: Add defensive checks before accessing client_state attributes in _handle_batch_mode_audio.

Lines 751-753 access client_state.user_id, client_state.user_email, and client_state.client_id without validation. While these are set during the audio-start message handler (lines 1342-1344) and the state machine enforces audio-start before audio-chunk processing can occur, defensive checks using hasattr or getattr would guard against protocol violations or future refactoring that might bypass the current ordering guarantee.

tests/resources/redis_keywords.robot-123-155 (1)

123-155: Make key selection deterministic when multiple keys match.
KEYS output order isn't guaranteed; picking the first match can lead to flaky assertions when multiple suffix keys exist. Sort the keys and, when expected_conversation_id is provided, prefer the key that matches the expected value instead of failing on the first key's mismatch.

💡 Suggested refinement (deterministic selection + expected value match)

@@
-    @{keys}=    Split String    ${result.stdout}    \n
-    ${keys_list}=    Evaluate    [k for k in ${keys} if k.strip()]
-    ${num_keys}=    Get Length    ${keys_list}
+    @{keys}=    Split String    ${result.stdout}    \n
+    ${keys_list}=    Evaluate    [k for k in ${keys} if k.strip()]
+    Sort List    ${keys_list}
+    ${num_keys}=    Get Length    ${keys_list}
@@
-    ${redis_key}=    Get From List    ${keys_list}    0
-    Log    Found Redis key: ${redis_key}
-
-    # Get the conversation_id value
-    ${conversation_id}=    Redis Command    GET    ${redis_key}
-
-    # Optionally verify it matches expected value
-    IF    '${expected_conversation_id}' != '${None}'
-        Should Be Equal As Strings    ${conversation_id}    ${expected_conversation_id}
-        ...    Redis key value mismatch: expected ${expected_conversation_id}, got ${conversation_id}
-    END
+    ${redis_key}=    Get From List    ${keys_list}    -1
+    ${conversation_id}=    Redis Command    GET    ${redis_key}
+
+    # If expected value provided, prefer the key that matches it
+    IF    '${expected_conversation_id}' != '${None}'
+        FOR    ${candidate}    IN    @{keys_list}
+            ${candidate_value}=    Redis Command    GET    ${candidate}
+            IF    '${candidate_value}' == '${expected_conversation_id}'
+                ${redis_key}=    Set Variable    ${candidate}
+                ${conversation_id}=    Set Variable    ${candidate_value}
+                Exit For Loop
+            END
+        END
+        Should Be Equal As Strings    ${conversation_id}    ${expected_conversation_id}
+        ...    Redis key value mismatch: expected ${expected_conversation_id}, got ${conversation_id}
+    END
+    Log    Found Redis key: ${redis_key}

backends/advanced/webui/src/pages/System.tsx-178-189 (1)

178-189: Avoid misc-settings calls for non-admin users.
/api/misc-settings is admin-only; calling it when isAdmin is false will just generate 403s and console noise.

✅ Minimal guard

   const loadMiscSettings = async () => {
+    if (!isAdmin) return
     try {
       setMiscLoading(true)
       const response = await systemApi.getMiscSettings()

backends/advanced/src/advanced_omi_backend/workers/audio_jobs.py-122-124 (1)

122-124: Remove unused f-string prefix.
Ruff F541: no interpolation is used here.

🔧 Quick fix

-        logger.info(f"🔍 always_persist=False - will wait for speech detection to create conversation")
+        logger.info("🔍 always_persist=False - will wait for speech detection to create conversation")

backends/advanced/webui/src/components/ConversationVersionDropdown.tsx-140-143 (1)

140-143: Guard against findIndex returning -1 (shows v0).

If the active id isn’t found, findIndex yields -1, resulting in a misleading v0. Add a guard (and consider a nullish check for numeric fallback).

🛠️ Suggested fix

+  const formatActiveVersionLabel = (
+    versions: Array<TranscriptVersion | MemoryVersion>,
+    activeId: string,
+    fallbackNumber?: number
+  ) => {
+    const idx = versions.findIndex(v => v.version_id === activeId)
+    if (idx >= 0) return `v${idx + 1}`
+    return fallbackNumber != null ? `v${fallbackNumber}` : '-'
+  }

...
-              Transcript: {versionHistory ?
-                `v${versionHistory.transcript_versions.findIndex(v => v.version_id === versionHistory.active_transcript_version) + 1}` :
-                (versionInfo?.active_transcript_version_number ? `v${versionInfo.active_transcript_version_number}` : '-')
-              }
+              Transcript: {versionHistory
+                ? formatActiveVersionLabel(
+                    versionHistory.transcript_versions,
+                    versionHistory.active_transcript_version,
+                    versionInfo?.active_transcript_version_number
+                  )
+                : (versionInfo?.active_transcript_version_number != null ? `v${versionInfo.active_transcript_version_number}` : '-')
+              }

...
-              Memory: {versionHistory ?
-                `v${versionHistory.memory_versions.findIndex(v => v.version_id === versionHistory.active_memory_version) + 1}` :
-                (versionInfo?.active_memory_version_number ? `v${versionInfo.active_memory_version_number}` : '-')
-              }
+              Memory: {versionHistory
+                ? formatActiveVersionLabel(
+                    versionHistory.memory_versions,
+                    versionHistory.active_memory_version,
+                    versionInfo?.active_memory_version_number
+                  )
+                : (versionInfo?.active_memory_version_number != null ? `v${versionInfo.active_memory_version_number}` : '-')
+              }

Also applies to: 202-205

backends/advanced/src/advanced_omi_backend/services/memory/providers/mcp_client.py-64-64 (1)

64-64: Make metadata explicitly optional.

Implicit Optional is flagged by type checkers/ruff. Use Optional[Dict[str, Any]] (or Dict[str, Any] | None) to satisfy PEP 484. Optional is already imported in the file.

♻️ Suggested fix

-    async def add_memories(self, text: str, metadata: Dict[str, Any] = None) -> List[str]:
+    async def add_memories(self, text: str, metadata: Optional[Dict[str, Any]] = None) -> List[str]:

tests/Makefile-244-259 (1)

244-259: Ensure API-key test target starts containers with the real config.

Line 244-259 runs tests without ensuring containers are started with deepgram-openai.yml. If test-no-api was run previously, this target will still hit mock services. Consider restarting containers with the real config in this target.

🛠️ Suggested adjustment

 test-with-api-keys:
 	`@echo` "🧪 Running tests that require API keys..."
 	`@if` [ -z "$$DEEPGRAM_API_KEY" ] || [ -z "$$OPENAI_API_KEY" ]; then \
 		echo "❌ Error: DEEPGRAM_API_KEY and OPENAI_API_KEY must be set"; \
 		echo "   export DEEPGRAM_API_KEY='your-key-here'"; \
 		echo "   export OPENAI_API_KEY='your-key-here'"; \
 		exit 1; \
 	fi
+	@$(MAKE) containers-stop
+	`@TEST_CONFIG_FILE`=/app/test-configs/deepgram-openai.yml $(MAKE) containers-start
 	CREATE_FIXTURE=true uv run --with-requirements test-requirements.txt robot --outputdir $(OUTPUTDIR) \
 		--name "API Key Tests" \
 		--console verbose \
 		--loglevel INFO:INFO \
 		--include requires-api-keys \
 		$(TEST_DIR)

backends/advanced/src/advanced_omi_backend/services/transcription/mock_provider.py (1)

54-56: Unused audio_duration variable.

The audio_duration is calculated but never used. Consider either removing it or using it to dynamically scale word timestamps based on actual audio length.

🧹 Suggested fix

-        # Calculate audio duration from bytes (assuming 16-bit PCM)
-        audio_duration = len(audio_data) / (sample_rate * 2)  # 2 bytes per sample
-
         # Return a mock transcript with word-level timestamps

Alternatively, if you want dynamic timing based on audio duration:

         # Calculate audio duration from bytes (assuming 16-bit PCM)
         audio_duration = len(audio_data) / (sample_rate * 2)  # 2 bytes per sample
+        # Scale factor to spread words across actual audio duration
+        scale = audio_duration / 6.5 if audio_duration > 0 else 1.0

backends/advanced/src/advanced_omi_backend/services/transcription/__init__.py (1)

73-77: Hardcoded fail_mode=False limits test flexibility.

The mock provider path always creates MockTranscriptionProvider(fail_mode=False), which means tests using RegistryBatchTranscriptionProvider with model_provider="mock" cannot trigger failure scenarios. The fail_mode parameter is only accessible through get_mock_transcription_provider().

If failure testing through the registry path is needed, consider reading fail_mode from configuration or environment.

💡 Optional: Support fail_mode through config or environment

         # Special handling for mock provider (no HTTP server needed)
         if self.model.model_provider == "mock":
             from .mock_provider import MockTranscriptionProvider
-            mock = MockTranscriptionProvider(fail_mode=False)
+            # Allow fail_mode to be configured for testing scenarios
+            import os
+            fail_mode = os.getenv("MOCK_TRANSCRIPTION_FAIL_MODE", "false").lower() == "true"
+            mock = MockTranscriptionProvider(fail_mode=fail_mode)
             return await mock.transcribe(audio_data, sample_rate, diarize)

backends/advanced/src/advanced_omi_backend/controllers/websocket_controller.py (3)

844-888: Improve exception handling in streaming validation.

Several exception handling issues:

1. Line 873: raise ValueError(error_msg) inside try block is caught by the except on line 877
2. Line 878: Use logging.exception instead of logging.error to capture stack trace
3. Line 880: Missing from err or from None to distinguish from exception handling errors

The control flow with catching ValueError to re-raise it is convoluted.

♻️ Suggested refactor for cleaner error handling

                     await websocket.send_json(error_response)
                     application_logger.info(f"📤 Sent streaming error to WebUI client {client_id}")

                     # Close the websocket connection after sending error
                     await websocket.close(code=1008, reason="Streaming transcription not configured")
                     application_logger.info(f"🔌 Closed WebSocket connection for {client_id} due to streaming config error")

-                    # Raise ValueError to exit the handler completely
-                    raise ValueError(error_msg)
-                except ValueError:
-                    # Re-raise ValueError to exit handler
-                    raise
+                    # Return with error indication to exit handler
+                    raise ValueError(error_msg) from None
                 except Exception as e:
-                    application_logger.error(f"Failed to send error to client: {e}")
-                    # Still raise ValueError to exit handler
-                    raise ValueError(error_msg)
+                    application_logger.exception(f"Failed to send error to client: {e}")
+                    raise ValueError(error_msg) from e

---

937-958: Unused parameter user_email and f-string without placeholders.

The user_email parameter is never used in _process_rolling_batch. Additionally, line 957 has an f-string without placeholders.

🧹 Suggested fix

 async def _process_rolling_batch(
     client_state,
     user_id: str,
-    user_email: str,
     client_id: str,
     batch_number: int
 ) -> None:
     """
     Process accumulated batch audio as a rolling segment.

     Creates conversation titled "Recording Part {batch_number}" and enqueues transcription.

     Args:
         client_state: Client state with batch_audio_chunks
         user_id: User ID
-        user_email: User email
         client_id: Client ID
         batch_number: Sequential batch number (1, 2, 3...)
     """
     if not hasattr(client_state, 'batch_audio_chunks') or not client_state.batch_audio_chunks:
-        application_logger.warning(f"⚠️ No audio chunks to process for rolling batch")
+        application_logger.warning("⚠️ No audio chunks to process for rolling batch")
         return

And update the call site at line 754-760:

         await _process_rolling_batch(
             client_state,
             user_id=client_state.user_id,
-            user_email=client_state.user_email,
             client_id=client_state.client_id,
             batch_number=client_state.batch_chunks_processed + 1
         )

---

986-994: Unused num_chunks variable.

The result of convert_audio_to_chunks is assigned but never used.

🧹 Suggested fix

         # Convert to MongoDB chunks
-        num_chunks = await convert_audio_to_chunks(
+        await convert_audio_to_chunks(
             conversation_id=conversation_id,
             audio_data=complete_audio,
             sample_rate=sample_rate,
             channels=channels,
             sample_width=width
         )

backends/advanced/docker-compose-test.yml (2)

141-141: Consider making CUDA version overrideable for non‑GPU setups.
An env‑level override keeps the profile usable on machines without compatible drivers.

♻️ Possible tweak

     args:
-        PYTORCH_CUDA_VERSION: cu12.6
+        PYTORCH_CUDA_VERSION: ${PYTORCH_CUDA_VERSION:-cu12.6}

---

168-193: Optional: gate mock services behind a profile.
If these mocks are only needed for specific suites, a profile can keep default runs lighter.

♻️ Example

   mock-streaming-stt:
     build:
       context: ../..
       dockerfile: tests/Dockerfile.mock-streaming-stt
@@
     restart: unless-stopped
+    profiles:
+      - mocks
@@
   mock-llm:
     build:
       context: ../..
       dockerfile: tests/Dockerfile.mock-llm
@@
     restart: unless-stopped
+    profiles:
+      - mocks

tests/resources/system_keywords.robot (1)

44-81: Avoid fixed sleep for mock server readiness.
A hard Sleep 2s can be flaky or wasteful; prefer polling for port readiness or a log line before proceeding.

backends/advanced/webui/src/components/plugins/EnvVarsSection.tsx (1)

66-77: Minor: Verify the env_var display renders correctly.

The template literal ${fieldSchema.env_var} on line 69 will render as the literal string (e.g., $DEEPGRAM_API_KEY). If the intent is to show just the variable name without the $ prefix, consider removing it. If displaying the shell-style variable reference is intentional, this is fine as-is.

tests/resources/conversation_keywords.robot (1)

36-48: Naming: Consider renaming to Validate Conversation Count By Client ID.

The keyword name suggests it waits/polls for a conversation, but it actually performs a single check and asserts. The actual waiting is delegated to Wait Until Keyword Succeeds in the calling tests. While this pattern works, the name could be clearer:

• Current: Wait For Conversation By Client ID (implies polling)
• Suggested: Validate Conversation Count By Client ID or Assert Conversation Exists By Client ID

This is a minor documentation concern since the docstring does mention "Polls until..." but the keyword itself doesn't implement polling.

tests/integration/always_persist_audio_tests.robot (1)

41-45: Consider: Fixed sleep in Test Cleanup may cause flakiness or slow tests.

The 2-second sleep allows the backend to finalize processing, but this could be either too short (causing flakiness if processing takes longer) or unnecessarily slow. Consider if there's a status to poll for, or if this delay is acceptable given test reliability requirements.

tests/libs/mock_streaming_stt_server.py (2)

152-153: Use logging.exception for better error context.

When catching exceptions, logging.exception automatically includes the stack trace, providing better debugging information.

Proposed fix

                 except json.JSONDecodeError:
-                    logger.error(f"Invalid JSON from {client_id}: {message}")
+                    logger.exception(f"Invalid JSON from {client_id}: {message}")

---

167-169: Remove extraneous f-string prefixes.

These strings have no placeholders, so the f prefix is unnecessary.

Proposed fix

     logger.info(f"Starting Mock Streaming STT Server on {host}:{port}")
-    logger.info(f"Deepgram-compatible nested response format")
-    logger.info(f"Speech detection: >2.0s duration, >5 words")
+    logger.info("Deepgram-compatible nested response format")
+    logger.info("Speech detection: >2.0s duration, >5 words")

backends/advanced/src/advanced_omi_backend/clients/audio_stream_client.py (2)

155-159: Remove debug print statements.

These print() statements duplicate the logger.info() calls and appear to be debug artifacts. Using only the logger is more appropriate for production code as it respects log levels and configuration.

Proposed fix

-        print(f"🔵 CLIENT: Sending audio-start message: {header}")
         logger.info(f"🔵 CLIENT: Sending audio-start message: {header}")
         await self.ws.send(json.dumps(header) + "\n")
-        print(f"✅ CLIENT: Sent audio-start with mode={recording_mode}, always_persist={always_persist}")
         logger.info(f"✅ CLIENT: Sent audio-start with mode={recording_mode}, always_persist={always_persist}")

---

464-466: Use logging.exception for better error context.

When logging in an exception handler, logging.exception automatically includes the stack trace, which aids debugging.

Proposed fix

             except Exception as e:
                 session.error = str(e)
-                logger.error(f"❌ CLIENT: Stream {stream_id} failed to start: {e}")
+                logger.exception(f"❌ CLIENT: Stream {stream_id} failed to start: {e}")

backends/advanced/src/advanced_omi_backend/utils/logging_utils.py (2)

94-97: Consider masking empty string secrets.

The condition if is_secret and value won't mask falsy values like empty strings (""), 0, or False. While this might be intentional (no need to mask empty values), it could leak information that a secret field is empty vs having a value.

If you want to mask all secret fields regardless of value:

♻️ Suggested change

-        if is_secret and value:
+        if is_secret and value is not None:

---

254-256: Inconsistent case sensitivity for secret attribute matching.

create_masked_repr uses case-sensitive matching (key in secret_attrs), while mask_dict uses case-insensitive matching (key.lower() in secret_fields_lower). This inconsistency could lead to secrets not being masked if the casing doesn't match exactly.

♻️ Suggested fix for consistency

+    secret_attrs_lower = {attr.lower() for attr in secret_attrs}
+
     for key in dir(obj):
         # Skip private/magic attributes and methods
         if key.startswith('_') or callable(getattr(obj, key)):
             continue
         
         value = getattr(obj, key)
         
         # Mask secret attributes
-        if key in secret_attrs:
+        if key.lower() in secret_attrs_lower:
             value_repr = f"'{mask}'"

backends/advanced/src/advanced_omi_backend/plugins/email_summarizer/email_service.py (1)

173-183: Add exception chaining to preserve stack trace.

The detailed error messages are helpful, but without exception chaining (from e), the original traceback is lost, making debugging harder. This is flagged by Ruff B904.

♻️ Suggested fix

         except smtplib.SMTPAuthenticationError as e:
-            raise Exception(f"SMTP Authentication failed for {self.username}. Check credentials. For Gmail, use an App Password instead of your regular password. Error: {str(e)}")
+            raise Exception(f"SMTP Authentication failed for {self.username}. Check credentials. For Gmail, use an App Password instead of your regular password. Error: {e!s}") from e
         except smtplib.SMTPConnectError as e:
-            raise Exception(f"Failed to connect to SMTP server {self.host}:{self.port}. Check host and port. Error: {str(e)}")
+            raise Exception(f"Failed to connect to SMTP server {self.host}:{self.port}. Check host and port. Error: {e!s}") from e
         except smtplib.SMTPServerDisconnected as e:
-            raise Exception(f"SMTP server disconnected unexpectedly. Check TLS settings (port 587 needs TLS, port 465 needs SSL). Error: {str(e)}")
+            raise Exception(f"SMTP server disconnected unexpectedly. Check TLS settings (port 587 needs TLS, port 465 needs SSL). Error: {e!s}") from e
         except TimeoutError as e:
-            raise Exception(f"Connection to {self.host}:{self.port} timed out. Check firewall/network settings. Error: {str(e)}")
+            raise Exception(f"Connection to {self.host}:{self.port} timed out. Check firewall/network settings. Error: {e!s}") from e
         except Exception as e:
-            raise Exception(f"SMTP connection test failed: {type(e).__name__}: {str(e)}")
+            raise Exception(f"SMTP connection test failed: {type(e).__name__}: {e!s}") from e

tests/libs/mock_llm_server.py (2)

302-308: Remove extraneous f-string prefixes.

Lines 303-308 use f-strings but contain no placeholders. Per Ruff F541, these should be plain strings.

♻️ Suggested fix

     logger.info(f"Starting Mock LLM Server on {host}:{port}")
-    logger.info(f"OpenAI-compatible endpoints:")
-    logger.info(f"  - POST /v1/chat/completions")
-    logger.info(f"  - POST /v1/embeddings")
-    logger.info(f"  - GET /v1/models")
-    logger.info(f"  - GET /health")
-    logger.info(f"Deterministic embeddings: 1536 dimensions")
+    logger.info("OpenAI-compatible endpoints:")
+    logger.info("  - POST /v1/chat/completions")
+    logger.info("  - POST /v1/embeddings")
+    logger.info("  - GET /v1/models")
+    logger.info("  - GET /health")
+    logger.info("Deterministic embeddings: 1536 dimensions")

---

256-282: Consider prefixing unused request parameter.

The request parameter is required by aiohttp's handler signature but unused in handle_models and handle_health. Prefixing with _ indicates intentional non-use and silences the linter.

♻️ Suggested fix

-async def handle_models(request: web.Request) -> web.Response:
+async def handle_models(_request: web.Request) -> web.Response:
     """Handle /v1/models endpoint."""
     ...

-async def handle_health(request: web.Request) -> web.Response:
+async def handle_health(_request: web.Request) -> web.Response:
     """Handle health check endpoint."""
     ...

backends/advanced/src/advanced_omi_backend/config.py (1)

207-231: Avoid partial writes when saving misc settings.

If one section saves and the other fails, the call returns False but state is already partially updated. Consider a single atomic save or returning per-section status/rollback details.